fix(macos): unbreak STT, the HUD, and the macOS package - #213
Merged
Conversation
…t its real device
Building the whisper.cpp helper on macOS produced a staged directory that only
worked on the machine that built it, and misreported what it had done.
Three defects, all in the blind spot left by unit tests that mock `fetch`:
- The sidecar glob (`ggml*.*`) matched only Windows DLL spelling. CMake emits
`libggml-base.dylib` / `libggml-base.so.0` elsewhere, so on macOS and Linux
every ggml library was skipped and the helper died in dyld before main().
It also swept up `.lib`/`.exp`/`.pdb` build by-products into the Windows
installer. The glob now covers all three platforms' naming (27/27 real
sidecars, 0 by-products) and `cp -a` keeps the version symlink farm instead
of dereferencing each link into a full copy.
- The staged binary's only LC_RPATH was an absolute path into
`.cache/whisper-stt-build`. Deleting the build cache — or downloading the CI
artifact onto another runner, which is what build.yml does — left dyld unable
to find libwhisper: SIGABRT, exit 134, before main(). Now rewritten to
`@loader_path` and re-signed ad-hoc.
- `detect_active_backend()` matched the string "Metal", but ggml's Metal
*device* is named from GGML_METAL_NAME ("MTL0"), so every Apple Silicon run
reported `whispercpp-cpu` while Metal was bound. It now matches MTL and
filters on device type GPU/IGPU, which also excludes ggml-blas.
Also: `detected_language` echoed the request parameter. The renderer always
sends "auto", so the field was permanently the literal "auto" — displayed in
the media stage's "detected language" line and written onto
AxcutTranscript.language. It now reports whisper_full_lang_id().
stage-whisper-stt.sh's load check could not catch any of this: it asserted the
helper printed *something*, which is a Windows-shaped test (that loader dies
mute) — dyld's four-line error satisfied it. It now requires the
`[whisper-stt] boot:` line main() emits, and no longer depends on `timeout`,
which is absent from a stock macOS runner and made the check pass without ever
starting the binary.
Adds scripts/test-whisper-stt.mjs (`npm run test:whisper-stt`), which
round-trips a real clip through a real helper and asserts the contract.
Verified on macOS arm64/Metal: WER 0.0000 on an English and a French clip,
rtf 0.18-0.25 on an M1, from a relocated copy with the build tree deleted.
Vulkan on Windows and Linux is unverified since the backend-detection change.
Fixes the `nproc` fallback that built with -j4 on every Mac.
…Electron"
The harness hardcoded ~/Library/Application Support/Electron, but the leaf of
app.getPath("userData") is app.getName() — "openscreen" in dev, the
productName once packaged. "Electron" is only what a bare `electron .` with
no app name uses, so the default pointed somewhere the app never writes. Probe
the plausible names and take the one that exists.
transcript.whisperHint told users "runs in your browser". Whisper runs in the main process: SttManager spawns the native whisper-stt-server helper (whisper.cpp linked statically) and talks to it over loopback HTTP. The renderer only ships it audio and maps the result back. See technical-documentation/architecture/transcription-and-captions.md. Reworded to "runs on your computer" (and the per-language equivalent) across all 13 locales. The "no data leaves the device" half was already accurate and is untouched — as is the rest of each sentence, since only the offending clause was replaced. "computer" rather than "device" because the second half already says data never leaves the device, and repeating it reads as a stutter in most of these languages. Only whisperHint made the claim; nothing else in the locales, the docs or the website says transcription is a browser feature. npm run i18n:check passes; the diff is 13 lines across 13 files.
… invisible On macOS 26, a window that has had setContentProtection(true) applied is never displayed. Not "excluded from screen captures", which is the documented behaviour and the reason we call it — never painted for the user either. The app started with a tray icon, a healthy renderer and no window on screen, and both the tray click and Cmd+Shift+O were inert because Electron already considered the window visible. Confirmed on macOS 26.5 / Electron 41.2.1 against the HUD: ready-to-show fired, show() was called, geometry was on-screen on the main display, React had mounted with no console errors. Unsetting protection makes it appear instantly. Reordering protect-vs-show changes nothing, so it is the call itself, not the timing. setContentProtection maps to NSWindow.sharingType = NSWindowSharingNone, a path Electron has churned on repeatedly (electron/electron#45990; PR #46886 reverted a macOS content-protection refactor). Gated on the macOS major version, not darwin wholesale: the breakage is only confirmed on 26.x, and dropping protection on older macOS where it may work would be a privacy regression made on no evidence. The check reads process.getSystemVersion() (macOS "26.5.0"), NOT os.release() (Darwin "25.5.0") — feeding it the latter would silently never match. OPENSCREEN_FORCE_CONTENT_PROTECTION=1 re-enables it to re-test against a future Electron. KNOWN TRADE-OFF: the HUD and Notes windows can now appear in captures on macOS 26. Apple already made that partly unavoidable — ScreenCaptureKit ignores sharingType, so any SCK-based recorder captures them regardless, including our own helper. The durable fix is to exclude our windows via SCContentFilter's excludingWindows:, which electron/native/screencapturekit currently passes as []. Not done here: it needs a recording to verify, which needs screen tooling I don't have in this session.
…inverted ranges Verified against a real 24.7s French screen recording (WER-free run: 10 segments, 57 words, fr detected, rtf 0.076 on M1/Metal). Two problems with the single bundled `every()`: - It could not say which property broke. The real clip tripped it and the output read "word times lie within the clip — clip is 24.70s" when nothing was outside the clip: max word end was 24.00s. The actual trigger was `end < start` on three tokens. - Asserting `end >= start` on the raw helper response is wrong for this layer. whisper.cpp gives a segment's last word the segment's own t1 as its end, while the word's DTW start runs 80-150ms late, so a token emitted near the boundary (standalone punctuation: "?", "!", "!") lands after it and inverts. That is anticipated and handled one layer up, on purpose — whisperServer.ts clamps to Math.max(startSec + 0.02, endSec), and snapWordBoundaries.ts keeps "degenerate words (whisper sometimes reports end <= start) non-empty". The document never sees an inverted range. Now three separate checks (starts non-negative, ends within clip, starts monotonic) plus an informational count of raw inversions.
…r addon cargo stamps the cdylib's own id (LC_ID_DYLIB) with the absolute path it was built at — crates/target/release/deps/libcompositor_view.dylib. That is not a load failure: require() dlopens the .node by path and never reads LC_ID_DYLIB, and every real dependency was already rewritten to @rpath. But it ships a build-machine path inside a release artefact, complete with the builder's home directory and checkout name, which defeats reproducible builds and leaks a filesystem layout to anyone running `otool -D` on the installed app. Set it to @rpath/compositor_view.node and re-sign. The existing post-condition could not have caught it: it grepped otool -L output for `lib(av|sw)` only, so it saw neither the id nor any future non-ffmpeg dependency arriving absolute. Replaced with the real invariant — no reference, id or dependency, may be an absolute path outside /usr/lib and /System — and it now runs over the vendored dylibs too, not just the addon. Verified: shipped copy's id is @rpath/compositor_view.node with a valid signature; the dev copy under electron/native/compositor-view/build/ still carries its absolute id, which the new check correctly identifies as the value it would reject (that copy never leaves the checkout, by design).
….app The macOS CI job built the ScreenCaptureKit helper and staged the whisper-stt binaries, but never ran fetch:ffmpeg:mac or build:native:compositor:mac. So the .app it produced had no compositor_view.node at all: preview and export dead in the installed app, with nothing in any log to say why. Windows does not drift this way because its job just runs `npm run build:win`, which chains both; the macOS job spells its steps out (it needs --dir plus a hand-rolled DMG and signing) and fell behind the `build:mac` recipe it was supposed to mirror. Nothing caught it, because before-pack.cjs — the hook written precisely to stop a broken compositor shipping — returned early on every non-win32 platform. Two changes: - build.yml gains the two missing steps, both cached: the LGPL ffmpeg tree (built from source, ~5 min, keyed on the fetch script so the cache busts when the pin moves) and the Metal addon. - before-pack.cjs now runs on darwin. It asserts the *shipped* payload in electron/native/bin/darwin-<arch>/ — the directory mac.extraResources copies wholesale — contains the compositor addon, the ffmpeg dylibs it links, the whisper-stt helper, the ggml dylibs that helper links, and the ScreenCaptureKit helper, naming what breaks for each. It then applies the existing staleness check to the arch-tagged addon rather than the dev copy, since the arch-tagged one is what actually gets packaged. Verified both directions: passes on a complete tree; with compositor_view.node removed it names the addon, with libggml*.dylib removed it names the ggml dylibs, exit 1 both times. build.yml re-parses as valid YAML and the macOS job's step order is unchanged apart from the insertions. NOT touched: the job's `if: false`. Its comment marks it release-branch-only for 1.8.0 shipping Windows-only, and says explicitly not to let it reach main. So these fixes make the job correct but do not make it run — that is a release decision, and it is now flagged in build-and-packaging.md.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Builds the whisper.cpp STT helper on macOS for the first time and makes the macOS deliverable actually complete. Seven fixes, each verified on real hardware (macOS 26.5, Apple M1, Electron 41.2.1).
STT was never built or run on macOS before this branch. Doing so surfaced three defects that would have shipped in a dmg:
ggml*.*, which matches Windows'ggml-base.dllbut notlibggml-base.dylib/libggml-base.so.0— so macOS and Linux staged onlylibwhisperand the helper died in dyld beforemain(). Only Windows ever worked. The glob also swept.lib/.exp/.pdbinto the Windows installer; the new one stages 27/27 real sidecars and no by-products.LC_RPATHwas an absolute path into.cache/whisper-stt-build. Deleting the build cache — or downloading the CI artifact onto another runner, which is whatbuild.ymldoes — gave SIGABRT beforemain(). Now rewritten to@loader_pathand re-signed.detect_active_backend()matched the string"Metal", but ggml's Metal device is namedMTL0. The field the contract calls the source of truth for which device ran was wrong on every Apple Silicon run.stage-whisper-stt.sh's load check could not catch any of this: it asserted the helper printed something, which is a Windows-shaped test (that loader dies mute) — dyld's four-line error satisfied it. It now requires the[whisper-stt] boot:linemain()emits, and no longer depends ontimeout(GNU coreutils, absent from a stock macOS runner, which made the check pass without ever starting the binary).Separately, the app did not start on macOS 26. A window that has had
setContentProtection(true)applied is never displayed there — not merely absent from captures, which is the documented behaviour, but never painted. The app came up with a tray icon, a live renderer and nothing on screen; the tray click and the global shortcut were both inert because Electron already considered the window visible. Gated on the macOS major version, notdarwinwholesale, since the breakage is only confirmed on 26.x.And the macOS package was incomplete. CI built the ScreenCaptureKit helper but never ran
fetch:ffmpeg:macorbuild:native:compositor:mac, so the.appshipped with no compositor addon — preview and export dead, silently. Nothing caught it becausebefore-pack.cjsreturned early on every non-win32 platform. Both fixed; the hook now asserts the whole shipped payload per platform.Related issue
Refs #212 — that one (Windows projects opening empty on macOS) is diagnosed but not fixed here.
Type of change
Release impact
Desktop impact
Verification
npm run test:whisper-stt(added here) round-trips a real clip through a real helper and asserts the contract — segments and per-word timings present, DTW guardrail passed, GPU offload reported on a GPU-capable host,detected_languageresolved rather than echoed, word times non-negative / in-clip / monotonic, and WER against a reference.PATH.backend=whispercpp-metal,auto-detected language: fr (p = 0.991),§4.1 guardrail: PASS (non_special_tokens=74).before-pack.cjstested both directions: passes on a complete tree; names the addon whencompositor_view.nodeis removed, names the ggml dylibs when those are removed, exit 1 both times.tsc --noEmit, Biome,i18n:check, andbuild.ymlre-parsed as valid YAML.Not verified
Vulkan0/CUDA0still match the same substring checks and the Windows staging path is untouched, but nobody has run it.libggml*.so*; the fix is pattern-tested against realistic filenames but was only built on macOS.@loader_pathmatches how electron-builder lays outresources/electron/native/bin/<tag>/, but no installer has been built and run — and the macOS job is stillif: false(release-branch-only for 1.8.0 shipping Windows-only), so these CI fixes make the job correct without making it run. That flag is deliberately untouched.